-
Notifications
You must be signed in to change notification settings - Fork 331
[CQ] remove experimental BadgeIcon
dependency
#8365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
import io.flutter.run.common.RunMode; | ||
import io.flutter.run.daemon.FlutterApp; | ||
|
||
import java.awt.Color; | ||
import java.awt.Paint; | ||
import javax.swing.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that we avoid * imports, but I'm not sure if that's enforced somewhere in java settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! This is what the IDE did on-save. Doing a quick grep I see we've got 104+ *
imports in 74+ files so we're definitely not consistently avoiding them.
That could be this inspection?
https://www.jetbrains.com/help/inspectopedia/OnDemandImport.html
Happy to discuss enabling!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick search suggests to me it is better to avoid * imports (avoids potential naming conflicts, avoids indexing classes that aren't needed). Is enabling the inspection something we do individually or can we save it to the repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking a bit about it, I tend to agree. Wildcard imports are probably best to avoid. Could you add some discussion to #8098?
We do have a checked in inspection set and think we should probably enable it there but we should discuss. It would also be great to get the inspections run in the CI, if not as part of the verifier, as a separate task. I've poked at that a bit but got distracted. (Some info here: https://www.jetbrains.com/help/idea/command-line-code-inspector.html#inspection-profiles.)
Reverts #8365. Fixes: #8422 --- <details> <summary>Contribution guidelines:</summary><br> - See our [contributor guide]([https://github.com/dart-lang/sdk/blob/main/CONTRIBUTING.md](https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview) for general expectations for PRs. - Larger or significant changes should be discussed in an issue before creating a PR. - Dart contributions to our repos should follow the [Dart style guide](https://dart.dev/guides/language/effective-dart) and use `dart format`. - Java and Kotlin contributions should strive to follow Java and Kotlin best practices ([discussion](#8098)). </details>
The class
com.intellij.ui.BadgeIcon
is marked@experimental
and is flagged by the verifier:This change introduces a lightweight replacement, allowing us to remove the dependency and enable
VerifyPluginTask.FailureLevel.EXPERIMENTAL_API_USAGES
failure checking in the verifier (see: #8361).Behavior is preserved w/ the replacement:
Default:
with badge:
Contribution guidelines:
dart format
.